Conversation
Unfortunately, libnvme follows the POSIX way to return errors. This means the return value is not containing the error code, it is in errno. While at it, also print the returned value only when debug mode is enabled. Signed-off-by: Daniel Wagner <[email protected]>
| if (flags != -EINVAL) | ||
| nvme_show_connect_msg(c, flags); | ||
| nvme_strerror(-ret)); | ||
| } else if (flags != -EINVAL) { |
There was a problem hiding this comment.
The existing flags checking not correct since never set -EINVAL to flags so always executed nvme_show_connect_msg() if ret set zero.
There was a problem hiding this comment.
Indeed, the flags check doesn't make sense. The flag can't be EINVAL, because we already filtered this out. It turns out that the device name is supposed to be printed on connect. I don't like it, IMO it should only printed with --verbose but it's too late to change this now. So lets keep it.
0ad49ad ("Print device name on connect")
There was a problem hiding this comment.
It turns out we can cleanup the error code path even more. Thanks for pointing it out!
There was a problem hiding this comment.
Thank you. The changes look good.
No worries. Since I wrote all the error handling code I should have spotted it. I had a very good reproducer |
With the introduction of nvme_add_ctrl we can use the return code directly and don't rely on the errno being set. Also we can directly leave the function on error because we don't have to cleanup things explicitly anymore. Signed-off-by: Daniel Wagner <[email protected]>
igaw
force-pushed
the
handle-add-ctrl-errors
branch
from
c3e7c61 to
1f1027a
Compare
2 participants
The previous 'fix' was not really working because the error status is in errno not in the return value.
fixes: #2801